Skip to content

Migrate HiveToDynamoDBOperator and SqlToS3Operator to use get_df#50126

Merged
eladkal merged 1 commit intoapache:mainfrom
guan404ming:migrate-amazon-transfer
May 8, 2025
Merged

Migrate HiveToDynamoDBOperator and SqlToS3Operator to use get_df#50126
eladkal merged 1 commit intoapache:mainfrom
guan404ming:migrate-amazon-transfer

Conversation

@guan404ming
Copy link
Member

@guan404ming guan404ming commented May 2, 2025 •
edited
Loading

Related Issue

Why

get_pandas_df deprecated in #48875, which would be replaced by get_df. Thus, we need to migrate them in providers.

How

  • This PR is focus on migration of Amazon/transfer
  • Migrate unit test
  • Refine the common-sql type

Note

There is some workaround for type here. It should be removed after the common-sql bundle the refined type definition. I've added the TODO in the comment and would get back here after the release.

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@guan404ming guan404ming force-pushed the migrate-amazon-transfer branch 2 times, most recently from 86d526e to 8a30a7f Compare May 2, 2025 17:53
@guan404ming guan404ming mentioned this pull request May 2, 2025
Closed
16 tasks
@guan404ming guan404ming marked this pull request as ready for review May 2, 2025 18:22
@guan404ming guan404ming force-pushed the migrate-amazon-transfer branch 2 times, most recently from 51667c8 to bc63964 Compare May 3, 2025 17:06
@guan404ming guan404ming mentioned this pull request May 4, 2025
Merged
@guan404ming guan404ming force-pushed the migrate-amazon-transfer branch from bc63964 to 98da798 Compare May 4, 2025 06:48
@guan404ming guan404ming force-pushed the migrate-amazon-transfer branch from 98da798 to 49c53c5 Compare May 5, 2025 10:00
@guan404ming
Copy link
Member Author

guan404ming commented May 5, 2025 •
edited
Loading

Failed since the hive also need the overload like something I added in common-sql. I would open PR to solve it later.

@guan404ming
Copy link
Member Author

guan404ming commented May 5, 2025

Here is the PR #50211

@guan404ming
Copy link
Member Author

guan404ming commented May 6, 2025 •
edited
Loading

Hi, I’ve tried to figure out why mypy passes locally but fails in CI: the CI pipeline seems not import (or install ?) the dependency (common-sql/sql.pyi) mypy needs for its checks. If you compare the local and CI error logs, you’ll see that with our new type overload the error disappears locally, but CI still reports it. I'm not sure that should we update the CI configuration to force import that file, or just add a # type: ignore here or is there any better way to solve this in Slack/transfer and this one?

Thanks in advance!

Local:
image

CI:
image

@guan404ming
Copy link
Member Author

guan404ming commented May 6, 2025 •
edited
Loading

When I keep searching the answer I find the type def did not really following the newest python typing spec, thus I update them in #50229. I think maybe it could solve the error here since it didn't rely on the pyi file but to directly write the overload in the .py which I think it must be imported here.

@potiuk potiuk force-pushed the migrate-amazon-transfer branch from 49c53c5 to 814d079 Compare May 6, 2025 14:09
@guan404ming
Copy link
Member Author

guan404ming commented May 6, 2025 •
edited
Loading

Mypy seems passed and even the Hive related error is passed. It seems like related to the change here #50198 right?

@guan404ming guan404ming force-pushed the migrate-amazon-transfer branch from 814d079 to 8ed059e Compare May 6, 2025 14:51
@guan404ming
Copy link
Member Author

guan404ming commented May 6, 2025

Pushed again to ensure it is stable.

@guan404ming guan404ming force-pushed the migrate-amazon-transfer branch from 8ed059e to 47bb0cf Compare May 6, 2025 14:53
@guan404ming
Copy link
Member Author

guan404ming commented May 6, 2025

Mypy works great and push again for unrelated ci error

@guan404ming @potiuk
Migrate Amazon/transfer to use get_df
c40b4be 
Co-authored-by: Jarek Potiuk <potiuk@apache.org>
@guan404ming guan404ming force-pushed the migrate-amazon-transfer branch from 47bb0cf to c40b4be Compare May 6, 2025 16:38
@eladkal eladkal changed the title Migrate Amazon/transfer to use get_df Migrate HiveToDynamoDBOperator and SqlToS3Operator to use get_df May 8, 2025
@eladkal eladkal merged commit 35186bc into apache:main May 8, 2025
68 checks passed
@guan404ming guan404ming deleted the migrate-amazon-transfer branch May 8, 2025 07:13
@guan404ming
Copy link
Member Author

guan404ming commented May 8, 2025

Thanks for the suggestions and review!

ayush3singh pushed a commit to ayush3singh/airflow that referenced this pull request May 8, 2025
@guan404ming @potiuk
Migrate HiveToDynamoDBOperator and SqlToS3Operator to use get_df (
e9ede2b 
apache#50126)

Co-authored-by: Jarek Potiuk <potiuk@apache.org>
@eladkal eladkal mentioned this pull request May 8, 2025
Closed
31 tasks
github-actions bot pushed a commit that referenced this pull request Aug 12, 2025
@guan404ming @potiuk
[v3-0-test] Migrate HiveToDynamoDBOperator and SqlToS3Operator to…
3eacbcc 
… use `get_df` (#50126)

(cherry picked from commit 35186bc)

Co-authored-by: Guan Ming(Wesley) Chiu <105915352+guan404ming@users.noreply.github.com>
Co-authored-by: Jarek Potiuk <potiuk@apache.org>
@github-actions
Copy link

github-actions bot commented Aug 12, 2025

Backport successfully created: v3-0-test

Status Branch Result
✅ v3-0-test PR Link

ashb pushed a commit that referenced this pull request Aug 12, 2025
@github-actions @guan404ming @potiuk
[v3-0-test] Migrate HiveToDynamoDBOperator and SqlToS3Operator to…
72d92d4 
… use `get_df` (#50126) (#54409)

* [v3-0-test] Migrate `HiveToDynamoDBOperator` and `SqlToS3Operator` to use `get_df` (#50126)
(cherry picked from commit 35186bc)

Co-authored-by: Guan Ming(Wesley) Chiu <105915352+guan404ming@users.noreply.github.com>
Co-authored-by: Jarek Potiuk <potiuk@apache.org>
meldiner pushed a commit to faros-ai/airflow that referenced this pull request Feb 6, 2026
@github-actions @guan404ming
@potiuk @meldiner
[v3-0-test] Migrate HiveToDynamoDBOperator and SqlToS3Operator to…
788b0c2 
… use `get_df` (apache#50126) (apache#54409)

* [v3-0-test] Migrate `HiveToDynamoDBOperator` and `SqlToS3Operator` to use `get_df` (apache#50126)
(cherry picked from commit 35186bc)

Co-authored-by: Guan Ming(Wesley) Chiu <105915352+guan404ming@users.noreply.github.com>
Co-authored-by: Jarek Potiuk <potiuk@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@potiuk potiuk potiuk approved these changes

@eladkal eladkal eladkal approved these changes

@o-nikolas o-nikolas Awaiting requested review from o-nikolas o-nikolas is a code owner

Assignees

No one assigned

Labels

area:providers provider:amazon AWS/Amazon - related issues provider:common-sql

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@guan404ming @potiuk @eladkal